Bring the max stacking depth limit back for marker timings#4898
Conversation
canova
force-pushed
the
marker-timing-depth-limit
branch
from
678dac8 to
921f56d
Compare
canova
changed the title
|
@julienw I did some more testing and it looks like it works well. I think it's safe to land, what do you think about it? |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #4898 +/- ##
==========================================
- Coverage 88.42% 88.42% -0.01%
==========================================
Files 304 304
Lines 27183 27186 +3
Branches 7330 7331 +1
==========================================
+ Hits 24037 24039 +2
- Misses 2928 2929 +1
Partials 218 218 ☔ View full report in Codecov by Sentry. |
| break; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
When we add a new markerTiming track, then you rely that the markerTiming entry is empty, and then that markerTiming.end[markerTiming.length - 1] will be undefined, but I think it would be good to be more explicit.
How about keeping the exact same logic to before, but simply add a guard after line 164:
if (markerTimingsForName.length >= MAX_STACKING_DEPTH) {
// There's too many markers around the same time already, let's ignore this marker.
continue;
}There was a problem hiding this comment.
Yeah, that's right that we can keep the same logic and add this guard. Initially I came back to using good-old for loop instead of find because sometimes I feel like find is actually slower :) But I did some testing now and it looks like for this case it actually makes it faster. So reverting this to find sounds like a better path forward. Updated the code.
canova
force-pushed
the
marker-timing-depth-limit
branch
from
921f56d to
46ad8e3
Compare
canova
force-pushed
the
marker-timing-depth-limit
branch
from
46ad8e3 to
e59c9b0
Compare
[@canova Nazım Can Altınova] Return null instead of throwing an error if it fails to get the resource name in active tab view (#4905) [@fqueze Florian Quèze] Improve the precision of the getFittedText implementation. (#4893) [@fqueze Florian Quèze] Show marker count next to the marker name in the marker chart when a marker is hovered. (#4892) [@mstange Markus Stange] Convert some code to use the non-inverted call node table even while we're showing the inverted call tree (Merge PR #4899) [@canova Nazım Can Altınova] Bring the max stacking depth limit back for marker timings (#4898) [@mstange Markus Stange] Various preparations in call tree code for fast inverted tree (Merge PR #4897) [@mstange Markus Stange] Move call node path/index conversion functions into CallNodeInfo (Merge PR #4896) [@mstange Markus Stange] Make mergeFunction fast (Merge PR #4895) [@gthb Gunnlaugur Thor Briem] feat: support from-url profiles in compare (#4875) And all the locale contributors: de: Michael Köhler el: Jim Spentzos en-GB: Ian Neal es-CL: ravmn fr: Théo Chevalier fy-NL: Fjoerfoks ia: Melo46 it: Francesco Lodolo [:flod] nl: Fjoerfoks, Mark Heijl pt-BR: Marcelo Ghelman ru: Valery Ledovskoy sv-SE: Andreas Pettersson uk: Artem Polivanchuk zh-CN: 你我皆凡人, Olvcpr423, Pontoon zh-TW: Pin-guang Chen
2 participants
This brings back the
MAX_STACKING_DEPTHthat was removed in 6920425 to make the marker chart more performant for cases where there are a lot of marker that are stacked on top of each other.For example:
Before / After
Before / After
Fixes #4808 and one of the profile is from #4894.